-
Notifications
You must be signed in to change notification settings - Fork 559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[finufft] match LLVM version of LLVMOpenMP #9423
Conversation
This PR fixes wrong test results in FINUFFT.jl on MacOS (see ludvigak/FINUFFT.jl#64)
Can you please elaborate on why this is necessary? It isn't immediately obvious to me. |
For whatever reason, the finufft library provided by finufft_jll yields wrong results on an M3 Macbook using a more recent LLVM version if more than ten threads are used (by default, finufft uses all available CPU cores, i.e., twelve threads on my M3 Pro). Accordingly, the tests performed by |
Nice catch, although a bit disturbing. What could be causing this? |
@jkrimmer just to be clear, you have verified this change solves your issue? I'm still very confused about what could be the cause of this. |
Unfortunately, I have absolutely no idea what could be causing this issue! |
What confuses me a lot is that llvmopenmp on aarch64-darwin does have a dependency on the specific version of the compiler used to build it, because we need compiler-rt, but that's statically linked, so I'm missing what should be the correlation with the compiler used to compile other code. |
Although I do not know if this issue is related to #9265, there is another problem with finufft on MacOS (at least on aarch64) that leads to segmentation faults in a threaded environment, see also ludvigak/FINUFFT.jl#63. Regarding the LLVM version for building finufft, I assume there have been improvements in Apple Silicon support in LLVM v15 that might lead to the erroneous behavior in combination with OpenMP. The wrong test results also do not occur if I build finufft locally with Apple clang version 15.0.0 (clang-1500.3.9.4). Honestly, my choice of using v"13.0.1+1" in particular is a bit hand-waving. |
Note that Apple's |
Thanks for the reminder; however, I have tested all LLVM versions in between v13 and v17. Thereby, I observed the issue with the Bootstrap images for v15, v16, and v17. |
This PR fixes wrong test results in FINUFFT.jl on MacOS (see ludvigak/FINUFFT.jl#64).